Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E2E tests #414

Merged
merged 81 commits into from
Feb 15, 2024
Merged

Conversation

phamminh0811
Copy link
Collaborator

Summary of changes

The e2e package defines an integration testing suite used for full end-to-end testing functionality. This package is decoupled from depending on the Terra codebase. It initializes the chains for testing via Docker files. As a result, the test suite may provide the desired Terra version to Docker containers during the initialization.

The file e2e_setup_test.go defines the testing suite and contains the core bootstrapping logic that creates a testing environment via Docker containers. A testing network is created dynamically with 2 test validators.

The file e2e_test.go contains the actual end-to-end integration tests that utilize the testing suite.

In E2E test, we added 3 tests which check the workflow of ibc-hooks, pfm and the original feetax design.

@StrathCole
Copy link
Collaborator

@StrathCole @nghuyenthevinh2000 we will remove the upgrade test cuz it is outside the scope outlined.

You mean the upgrade handler is out of scope? Because without the upgrade handler this can't be used in production as the chain upgrade would fail.

@phamminh0811
Copy link
Collaborator Author

@StrathCole I mean the upgrade test is not in the current scope that we proposed. Of course we will do that, but it will be in an another proposal that we gonna submit to community.

@StrathCole
Copy link
Collaborator

@StrathCole I mean the upgrade test is not in the current scope that we proposed. Of course we will do that, but it will be in an another proposal that we gonna submit to community.

Just to make sure we don't misunderstand each other. I am not referring to the upgrade test, I mean the upgrade handler to register the key store you are using with PFM/interchain.
Because when you run a chain with v2.3.3 and then upgrade to code that uses the changes from your PRs, it doesn't start.

So if I do
git checkout v2.3.3
make localnet-start
sudo systemctl restart docker (only due to a local iptables issue)
docker-compose up
—- wait until a few blocks are produced
git checkout branch-containing-pfm-interchain
make build-linux
docker-compose up

Then I end up with an error due to the missing store.
grafik

@expertdicer
Copy link
Collaborator

@StrathCole I mean the upgrade test is not in the current scope that we proposed. Of course we will do that, but it will be in an another proposal that we gonna submit to community.

Just to make sure we don't misunderstand each other. I am not referring to the upgrade test, I mean the upgrade handler to register the key store you are using with PFM/interchain. Because when you run a chain with v2.3.3 and then upgrade to code that uses the changes from your PRs, it doesn't start.

So if I do git checkout v2.3.3 make localnet-start sudo systemctl restart docker (only due to a local iptables issue) docker-compose up —- wait until a few blocks are produced git checkout branch-containing-pfm-interchain make build-linux docker-compose up

Then I end up with an error due to the missing store. grafik

That's might because we haven't had the v7 upgrade (or v6.2) which includes new added store for ibc-hooks and pfm, and the scripts above of yours didn't include the gov for the v7 upgrade.

@StrathCole
Copy link
Collaborator

That's might because we haven't had the v7 upgrade (or v6.2) which includes new added store for ibc-hooks and pfm, and the scripts above of yours didn't include the gov for the v7 upgrade.

That was exactly what I meant 😃
The upgrade handler wasn't yet included. I think that's the same @nghuyenthevinh2000 referred to.

@expertdicer
Copy link
Collaborator

@StrathCole I mean the upgrade test is not in the current scope that we proposed. Of course we will do that, but it will be in an another proposal that we gonna submit to community.

Just to make sure we don't misunderstand each other. I am not referring to the upgrade test, I mean the upgrade handler to register the key store you are using with PFM/interchain. Because when you run a chain with v2.3.3 and then upgrade to code that uses the changes from your PRs, it doesn't start.

So if I do git checkout v2.3.3 make localnet-start sudo systemctl restart docker (only due to a local iptables issue) docker-compose up —- wait until a few blocks are produced git checkout branch-containing-pfm-interchain make build-linux docker-compose up

Then I end up with an error due to the missing store. grafik

I have added the v7 upgrade handler and tested locally

image
image
image

@StrathCole
Copy link
Collaborator

StrathCole commented Feb 1, 2024

I have added the v7 upgrade handler and tested locally

@expertdicer looks like upgrade test now also passes 👍

tests/e2e/containers/containers.go Show resolved Hide resolved
tests/e2e/e2e_test.go Outdated Show resolved Hide resolved
tests/e2e/e2e_test.go Show resolved Hide resolved
tests/e2e/e2e_test.go Outdated Show resolved Hide resolved
tests/e2e/initialization/config.go Outdated Show resolved Hide resolved
tests/e2e/initialization/config.go Outdated Show resolved Hide resolved
@nghuyenthevinh2000
Copy link
Contributor

nghuyenthevinh2000 commented Feb 4, 2024

I find these three PRs:

  1. E2E tests #414
  2. Feat/interchaintest #402
  3. Terra localnet #405

containing overlapped logic to test pfm and ibc - hooks, so three times in total. Can you guys close two others and use this instead? Please provide reasons if this is not the case

@phamminh0811
Copy link
Collaborator Author

@nghuyenthevinh2000
Compared to E2E, the Interchaintest will look more closely at ibc activities. For instance, it has various relayer options and can pull other chain images, such as those from Osmosis and Gaia, while E2E can't. Therefore, in our opinion, each of the two tests ought to exist separately.

You are free to close the localnet one, this is merely our first step toward getting ready for the tests and is not part of our PPJ scope.

@fragwuerdig
Copy link
Collaborator

will be merging - snyk test will be handled in base branch

@fragwuerdig fragwuerdig merged commit a5d31c6 into classic-terra:frag/foundation Feb 15, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants